docs/notes/pipeline-usage.md-32-35 (1)

32-35: ⚠️ Potential issue | 🟡 Minor

Priority collision: RateLimitFilter and JwtAuthFilter share priority 100 on /api/*

Both filters are documented with the same priority (100) for the /api/* path pattern:

pipeline.addRouteFilter(new RateLimitFilter(1000, 60000), 100, "/api/*");   // priority 100
pipeline.addRouteFilter(new JwtAuthFilter(),              100, "/api/*");   // priority 100

Same-priority filters on the same route create undefined execution order. The typical intent is rate-limiting before auth (so unauthenticated requests still consume their rate-limit budget and expensive auth logic isn't invoked for blocked clients). If your Pipeline uses stable insertion-order tie-breaking this may work today, but it's a fragile assumption. Consider separating the priorities explicitly, for example:

-        // pipeline.addRouteFilter(new RateLimitFilter(1000, 60000), 100, "/api/*");
+        // pipeline.addRouteFilter(new RateLimitFilter(1000, 60000),  90, "/api/*");  // runs before auth
         // pipeline.addRouteFilter(new JwtAuthFilter(), 100, "/api/*");

A brief comment explaining the priority ordering convention (lower = earlier vs. higher = earlier) would also prevent future misconfigurations.

Also applies to: 48-50

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/notes/pipeline-usage.md` around lines 32 - 35, Two filters
(RateLimitFilter and JwtAuthFilter) are shown with the same priority (100) for
the same route pattern ("/api/*"), causing undefined ordering; change the
pipeline.addRouteFilter calls so RateLimitFilter has a higher precedence than
JwtAuthFilter (e.g., set RateLimitFilter priority to run before JwtAuthFilter)
and add a short comment near the pipeline registration documenting the priority
convention used (explicitly state whether lower numbers run earlier or later) to
avoid future collisions; update both occurrences referenced (the /api/*
registrations around the earlier and later blocks) to use distinct priorities
and the new comment.

docs/notes/pipeline-usage.md-25-25 (1)

25-25: ⚠️ Potential issue | 🟡 Minor

Clarify or remove the "/admin/**" recursive wildcard caveat

The (if implemented) qualifier signals that this pattern may silently not work in the current router. A developer who uncomments a filter using "/admin/**" expecting recursive matching could end up with either no matches or subtly wrong behavior.

Either confirm implementation and remove the qualifier, or replace the example with a supported pattern.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/notes/pipeline-usage.md` at line 25, The docs note uses the ambiguous
pattern "/admin/**" with an "(if implemented)" caveat which risks misleading
callers; either confirm and remove the qualifier by updating the docs to assert
that "/admin/**" is supported, or replace the example with a supported pattern
(e.g., "/admin/*" or whatever the router actually matches) and remove the
parenthetical; locate the string "/admin/**" in pipeline-usage.md and update the
example and surrounding sentence so it accurately reflects the router's
implemented matching behavior.

Dockerfile-15-17 (1)

15-17: ⚠️ Potential issue | 🟡 Minor

Add an EXPOSE directive and resolve the hardcoded JAR name

Two minor gaps in the runtime stage:

1. No EXPOSE instruction — the listening port isn't self-documented in the image. Add EXPOSE <port> to signal the expected port to tooling and operators.
2. The hardcoded app.jar path (acknowledged in the TODO comment) must match the finalName in pom.xml (e.g., from the Maven Shade plugin). If they diverge the COPY will silently succeed but the entrypoint will fail at runtime with Unable to access jarfile app.jar.

Would you like me to verify the finalName configured in your pom.xml and generate the corrected COPY path?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 15 - 17, Add an EXPOSE line to document the service
port (e.g., EXPOSE 8080 or the actual port your app listens on) and make the
runtime-stage JAR path consistent with your Maven build: check the Maven pom.xml
(finalName or the Shade plugin’s output name) and update the COPY --from=build
source to that exact artifact (or make ENTRYPOINT reference the artifact name
you copy) so the jar path used by COPY and ENTRYPOINT match and avoid "Unable to
access jarfile app.jar" at runtime.

.github/workflows/ci.yml-14-14 (1)

14-14: ⚠️ Potential issue | 🟡 Minor

actions/checkout@v5 is outdated and inconsistent with docker-release.yml

The current latest version of actions/checkout is v6, while this workflow pins v5. The Docker release workflow in this same PR uses actions/checkout@v6.0.2, creating an inconsistency between the two CI workflows.

🔧 Proposed fix

-        uses: actions/checkout@v5
+        uses: actions/checkout@v6

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml at line 14, The workflow pins actions/checkout@v5
which is outdated and inconsistent with the other CI workflow; update the
checkout action in the CI workflow by replacing actions/checkout@v5 with the
current version used elsewhere (e.g., actions/checkout@v6.0.2) so both CI and
docker-release workflows use the same actions/checkout version; ensure any
required inputs remain compatible with the newer action.

src/main/resources/static/README.md-134-145 (1)

134-145: ⚠️ Potential issue | 🟡 Minor

Port mismatch between README and configuration.

The README documents port 3000 (lines 134, 143, 145, 163), but src/main/resources/application-properties.yml configures port: 8080. One of these needs to be updated to match the other to avoid confusing users.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/README.md` around lines 134 - 145, The README
documents the app running on port 3000 while the application config sets port:
8080, causing a mismatch; pick the intended runtime port and make them
consistent by either updating the README to show the configured port (change
documented URLs and docker run -p mapping from 3000 to 8080) or updating the
application configuration (change port: 8080 to port: 3000) so the documented
URL and docker run port mapping match the configured server port; ensure
references to the port value (e.g., the docker run -p mapping and the README
URL) and the application property "port: 8080" are updated together.

src/main/java/org/juv25d/http/HttpParser.java-61-61 (1)

61-61: ⚠️ Potential issue | 🟡 Minor

readNBytes may return fewer bytes than requested without error.

If the connection is closed prematurely, readNBytes(length) returns a shorter array silently. The parsed HttpRequest would then contain a truncated body with no indication of the problem. Consider validating body.length == length.

Proposed fix

         body = in.readNBytes(length);
+        if (body.length != length) {
+            throw new IOException("Incomplete body: expected " + length + " bytes, got " + body.length);
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/http/HttpParser.java` at line 61, The call to
in.readNBytes(length) in HttpParser can return fewer bytes than requested if the
connection closes; update the parsing code that assigns body to validate that
body.length == length after the read, and treat a short read as an error: throw
or return a failed parse (e.g., IllegalStateException or a ParseException)
and/or log and close the connection. Locate the assignment to body (body =
in.readNBytes(length)) in class HttpParser and add a check for the expected
length, handling the mismatch by failing the request parse rather than accepting
a truncated body.

src/main/resources/static/readme.html-18-18 (1)

18-18: ⚠️ Potential issue | 🟡 Minor

HTML entities missing terminating semicolons.

&#128269 and &#128270 should be 🔍 and 🔎. Without the semicolons, these are technically malformed character references per the HTML5 spec and may not render in strict parsers.

🔧 Proposed fix

-        <h1>&#128269 ReadMe &#128270</h1>
+        <h1>&#128269; ReadMe &#128270;</h1>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/readme.html` at line 18, The HTML character
references in the H1 element are missing terminating semicolons; update the H1
text node that currently contains "&#128269" and "&#128270" to use "🔍"
and "🔎" respectively so the character references are well-formed (locate
the <h1> element containing these entities and add the semicolons).

src/main/resources/static/readme.html-37-37 (1)

37-37: ⚠️ Potential issue | 🟡 Minor

Inconsistent script path — missing leading / on purify.min.js.

All other static assets on this page use root-relative paths (/js/..., /css/...), but purify.min.js uses a bare-relative path. While functionally identical when served from /, it will silently fail if the file is ever moved or served from a subdirectory.

🔧 Proposed fix

-<script src="js/purify.min.js"></script>
+<script src="/js/purify.min.js"></script>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/readme.html` at line 37, Update the script src for
purify.min.js to use a root-relative path so it matches the other static asset
URLs; locate the <script> tag that references "purify.min.js" and change its src
from "js/purify.min.js" to "/js/purify.min.js" to avoid breakage when the site
is served from a subdirectory.

src/test/java/org/juv25d/plugin/NotFoundPluginTest.java-24-24 (1)

24-24: ⚠️ Potential issue | 🟡 Minor

Specify charset explicitly in getBytes().

"404 - Resource Not Found".getBytes() uses Charset.defaultCharset(). If the JVM default is not UTF-8, this assertion can produce a false positive or false negative. Pair it with the same fix in NotFoundPlugin.java.

🔧 Proposed fix

-        assertArrayEquals("404 - Resource Not Found".getBytes(), res.body());
+        assertArrayEquals("404 - Resource Not Found".getBytes(StandardCharsets.UTF_8), res.body());

Also add the import at the top of the file:

+import java.nio.charset.StandardCharsets;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/plugin/NotFoundPluginTest.java` at line 24, Update
the assertion in NotFoundPluginTest to use an explicit charset rather than the
platform default: change the hardcoded byte comparison to use "404 - Resource
Not Found".getBytes(StandardCharsets.UTF_8) and add the corresponding import for
java.nio.charset.StandardCharsets; also make the analogous change in
NotFoundPlugin (where the response body is produced) to use
StandardCharsets.UTF_8 so both sides use the same encoding for res.body()
comparisons.

src/main/java/org/juv25d/plugin/NotFoundPlugin.java-13-13 (1)

13-13: ⚠️ Potential issue | 🟡 Minor

Specify charset in getBytes().

"404 - Resource Not Found".getBytes() uses Charset.defaultCharset(), which is platform-dependent. This can produce unexpected bytes on non-UTF-8 JVMs, and creates a fragile pairing with the test assertion in NotFoundPluginTest.

🔧 Proposed fix

+import java.nio.charset.StandardCharsets;
 ...
-        res.setBody("404 - Resource Not Found".getBytes());
+        res.setBody("404 - Resource Not Found".getBytes(StandardCharsets.UTF_8));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/plugin/NotFoundPlugin.java` at line 13, The response
body in NotFoundPlugin uses platform-default bytes via "404 - Resource Not
Found".getBytes(); change this to use an explicit charset (e.g.,
StandardCharsets.UTF_8) so the bytes are deterministic across platforms and
match tests: update the call in NotFoundPlugin (where res.setBody(...) is
invoked) to encode the string with StandardCharsets.UTF_8.

src/test/java/org/juv25d/logging/ConnectionIdLoggingTest.java-33-47 (1)

33-47: ⚠️ Potential issue | 🟡 Minor

Add logger.removeHandler(handler) to the finally block to prevent handler accumulation.

Logger.getLogger("test.connectionid") returns a shared, JVM-scoped instance. Without removing the handler, each test execution appends a new handler to the same logger. In parallel or repeated test-suite runs this causes handler leakage — prior run's handlers still capture records — making the test flaky.

🛡️ Proposed fix

         try {
             String testId = "test-123";
             LogContext.setConnectionId(testId);

             // Act
             logger.info("This is a test message");

             // Assert
             assertTrue(formattedMessages.get(0).contains("[" + testId + "]"), 
                 "Log message should contain the connection ID. Found: " + formattedMessages.get(0));
         } finally {
+            logger.removeHandler(handler);
             LogContext.clear();
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/logging/ConnectionIdLoggingTest.java` around lines
33 - 47, The test is leaking handlers on the shared Logger instance; modify the
finally block in ConnectionIdLoggingTest so it not only calls LogContext.clear()
but also removes the test handler from the logger by calling
logger.removeHandler(handler) to prevent handler accumulation across test runs;
locate the try/finally around LogContext.setConnectionId/test logging and ensure
you remove the same `handler` that was added to `logger` before clearing the
context so subsequent tests don't receive stale records from
`formattedMessages`.

src/main/java/org/juv25d/plugin/HelloPlugin.java-11-13 (1)

11-13: ⚠️ Potential issue | 🟡 Minor

Placeholder handle() produces an uninitialized HttpResponse (statusCode 0, statusText null).

Until the body is implemented, any request routed to HelloPlugin will emit a malformed HTTP response. Even for scaffolding, a minimal fallback (e.g., 200 OK with a placeholder body) prevents breaking the server during integration testing.

💡 Minimal placeholder implementation

     `@Override`
     public void handle(HttpRequest req, HttpResponse res) throws IOException {
-        // placeholder response logic
+        res.setStatusCode(200);
+        res.setStatusText("OK");
+        res.setBody("Hello, World!".getBytes(java.nio.charset.StandardCharsets.UTF_8));
+        res.setHeader("Content-Type", "text/plain; charset=UTF-8");
     }

Would you like me to generate a more complete implementation for this plugin?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/plugin/HelloPlugin.java` around lines 11 - 13, The
handle(HttpRequest req, HttpResponse res) method in HelloPlugin currently leaves
HttpResponse uninitialized, producing statusCode 0 and statusText null; update
HelloPlugin.handle to set a minimal fallback response (e.g., set status code to
200, status text to "OK", appropriate Content-Type, and a small placeholder body
like "Hello from HelloPlugin") and ensure headers and body length are correctly
set on res so any routed request returns a well-formed HTTP response during
integration tests.

src/main/resources/static/css/styles.css-63-132 (2)

63-132: ⚠️ Potential issue | 🟡 Minor

declaration-empty-line-before Stylelint errors throughout .nav-menu block — multiple [error] violations.

Stylelint 17.3.0 flags empty lines immediately before declarations on lines 65, 72, 88, 90–105. These should be resolved by removing the blank lines or adjusting the block-level comment placement to separate concerns without triggering the rule.

The most impactful instances (lines 72, 88, 90–105) can be addressed by moving anchor-name and position-anchor to immediately follow the previous declaration:

📐 Representative fix for lines 63–75

 .nav-menu {
-
     width: fit-content;
     margin: 0.4rem auto 1rem auto;
     display: flex;
     align-content: center;
     justify-content: center;
     isolation: isolate;
-
     anchor-name: --hovered-link;

Similar blank-line removal is needed at lines 88, 183, 212, 226, 228, 230–231, and 246 per the full Stylelint report.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/css/styles.css` around lines 63 - 132, Stylelint is
flagging empty lines before declarations inside the .nav-menu block; remove the
blank lines that precede declarations (or reposition block-level comments) so
declarations are contiguous—specifically move anchor-name: --hovered-link; and
position-anchor: --hovered-link; to immediately follow the preceding declaration
in the .nav-menu rule and in the &::before / &::after rules, and eliminate the
stray blank lines before other properties in ::before, ::after and
&:has(a:hover)::before / ::after so no declaration is separated by an empty
line.

---

63-132: ⚠️ Potential issue | 🟡 Minor

Nav hover effect uses CSS Anchor Positioning without an @supports guard.

The .nav-menu and .features blocks use anchor-name, position-anchor, and anchor() (lines 72, 75, 88, 122–125, 213, 215, 228) without @supports guards. As of February 2026, anchor-name and the anchor() function are Baseline Newly available, but position-anchor remains in limited availability. In Firefox, the feature is enabled by default only in version 147+; Firefox 145–146 requires a flag. Wrapping these declarations in @supports (position-anchor: top) or similar (as Chrome's documentation recommends) makes the intent explicit and future-proof, ensuring the decorative hover effect gracefully degrades in unsupported browsers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/css/styles.css` around lines 63 - 132, Wrap all
anchor positioning properties and anchor() usage inside a feature-detection
block so they gracefully degrade: add an `@supports` (position-anchor: top) guard
around the rules in .nav-menu that use anchor-name, position-anchor, and the
anchor() function (including the li:hover anchor-name, the &::before/&::after
top/left/right/bottom values that call anchor(), and the &:has(a:hover) block)
so those declarations only apply when position-anchor is supported and browsers
without it ignore them.

src/main/resources/static/css/styles.css-108-115 (1)

108-115: ⚠️ Potential issue | 🟡 Minor

Stylelint comment-whitespace-inside errors on lines 111 and 156 — add spaces inside comment delimiters.

Stylelint reports these as [error]-level violations.

🛠️ Proposed fix

-        /*backdrop-filter: blur(2px);*/
+        /* backdrop-filter: blur(2px); */

-    }*/
+    } */

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/css/styles.css` around lines 108 - 115, Stylelint
flags comment-whitespace-inside for comments inside the &::before and &::after
blocks; update inline CSS comments to include a single space after /* and before
*/ (e.g., change /*backdrop-filter: blur(2px);*/ to /* backdrop-filter:
blur(2px); */) and apply the same spacing convention to the other commented line
referenced in the &::after block so both comments conform to the
comment-whitespace-inside rule.

src/main/resources/static/index.html-47-47 (1)

47-47: ⚠️ Potential issue | 🟡 Minor

Missing leading / in purify.min.js script path.

All other static asset references (/css/styles.css, /js/marked.min.js, /js/app.js) use absolute paths. This one uses a relative path, which works when served from / but silently breaks under any sub-path routing or redirect.

🐛 Proposed fix

-<script src="js/purify.min.js"></script>
+<script src="/js/purify.min.js"></script>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/resources/static/index.html` at line 47, The script tag referencing
purify.min.js in index.html uses a relative path "js/purify.min.js" which breaks
under sub-path routing; update the script's src attribute so it uses an absolute
path starting with a leading slash (consistent with the other assets like
/css/styles.css and /js/app.js) to point to /js/purify.min.js.

src/main/java/org/juv25d/handler/StaticFileHandler.java-16-22 (1)

16-22: ⚠️ Potential issue | 🟡 Minor

Javadoc describes the resource root as /resources/static/ but the code maps to /static/.

STATIC_DIR = "/static/" is the correct classpath path (Maven strips the src/main/resources prefix). The Javadoc examples (GET / → /resources/static/index.html) are misleading.

📝 Suggested correction

- * Handles serving static files from the /resources/static/ directory.
+ * Handles serving static files from the classpath path /static/
+ * (corresponding to src/main/resources/static/ in the Maven source tree).
  * <p>
  * URL mapping:
- * - GET / → /resources/static/index.html
- * - GET /about.html → /resources/static/about.html
- * - GET /css/styles.css → /resources/static/css/styles.css
+ * - GET / → /static/index.html
+ * - GET /about.html → /static/about.html
+ * - GET /css/styles.css → /static/css/styles.css

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/handler/StaticFileHandler.java` around lines 16 -
22, The Javadoc incorrectly refers to "/resources/static/" while the code uses
STATIC_DIR = "/static/"; update the Javadoc in StaticFileHandler to reference
the actual classpath root "/static/" and adjust the URL mapping examples (e.g.,
"GET / → /static/index.html", "GET /about.html → /static/about.html", "GET
/css/styles.css → /static/css/styles.css") so they match the STATIC_DIR constant
and real runtime behavior.

src/test/java/org/juv25d/handler/StaticFileHandlerTest.java-13-20 (1)

13-20: ⚠️ Potential issue | 🟡 Minor

Tests asserting 200 depend on actual classpath resources—brittle and prone to silent failures.

shouldReturn200ForExistingFile, shouldReturnCorrectContentTypeForHtml/Css/Js, shouldServeIndexHtmlForRootPath, and shouldReturnNonEmptyBodyForSuccessfulRequest all invoke StaticFileHandler.handle and assert 200 responses. They depend on real files at src/main/resources/static/index.html, css/styles.css, and js/app.js. If these files are deleted or renamed, tests will fail silently with 404 assertions rather than signaling test framework errors.

Use either (a) test-only resource fixtures under src/test/resources/static/, or (b) mock the resource loading path to decouple tests from filesystem state.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/handler/StaticFileHandlerTest.java` around lines 13
- 20, The tests (shouldReturn200ForExistingFile,
shouldReturnCorrectContentTypeForHtml/Css/Js, shouldServeIndexHtmlForRootPath,
shouldReturnNonEmptyBodyForSuccessfulRequest) are brittle because they rely on
real classpath files; update the tests to use test fixtures or mocking: either
add the required files (index.html, css/styles.css, js/app.js) under
src/test/resources/static/ so StaticFileHandler.handle(request) finds them
during tests, or refactor the handler to allow injecting a
ResourceLoader/ClasspathResolver and in tests mock that dependency to return
controlled InputStreams for those paths; make the change against the test
methods and StaticFileHandler so assertions asserting 200 no longer depend on
production resources.

src/test/java/org/juv25d/handler/StaticFileHandlerTest.java-60-62 (1)

60-62: ⚠️ Potential issue | 🟡 Minor

new String(response.body()) without specifying charset.

The response body is UTF-8 encoded (set explicitly in StaticFileHandler). Relying on the platform default charset makes these assertions non-deterministic on systems where the default is not UTF-8.

♻️ Proposed fix (applies to lines 61, 62, and 72)

-        assertThat(new String(response.body())).contains("404");
-        assertThat(new String(response.body())).contains("Not Found");
+        assertThat(new String(response.body(), StandardCharsets.UTF_8)).contains("404");
+        assertThat(new String(response.body(), StandardCharsets.UTF_8)).contains("Not Found");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/handler/StaticFileHandlerTest.java` around lines 60
- 62, In StaticFileHandlerTest update the assertions that build Strings from
response.body() to specify UTF-8 explicitly (e.g., replace occurrences of new
String(response.body()) with new String(response.body(),
StandardCharsets.UTF_8)) so the tests are deterministic; ensure you import
java.nio.charset.StandardCharsets and apply this change for the assertions
checking "404", "Not Found" and any other place (line with 72) that converts
response.body() to a String.

src/main/java/org/juv25d/filter/RateLimitingFilter.java-109-118 (1)

109-118: ⚠️ Potential issue | 🟡 Minor

Client IP leaked in 429 response body.

Line 110 includes the client's IP address in the HTTP response body. This is a minor information disclosure — the client (or any intermediary) learns how the server perceives their IP. Remove the IP from the user-visible response and keep it in the server-side log only (which you already do in logRateLimitExceeded).

Proposed fix

-        byte[] body = ("429 Too Many Requests: Rate limit exceeded for IP " + ip + "\n")
+        byte[] body = "429 Too Many Requests: Rate limit exceeded\n"
             .getBytes(StandardCharsets.UTF_8);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/filter/RateLimitingFilter.java` around lines 109 -
118, The response body in sendTooManyRequests currently leaks the client's IP;
change the user-visible message to a generic text (e.g., "429 Too Many Requests:
Rate limit exceeded") and remove the ip variable from the body while leaving
headers/status intact; keep the existing logRateLimitExceeded call (or ensure
server-side logging still records the ip) so the IP is only present in server
logs, not in the HTTP response body.

src/main/java/org/juv25d/filter/RateLimitingFilter.java-83-91 (1)

83-91: ⚠️ Potential issue | 🟡 Minor

Use the builder API instead of the deprecated Bandwidth.classic() factory.

The project uses Bucket4j 8.16.1. While Refill.intervally() is still supported, Bandwidth.classic() is deprecated in 8.x. Replace the deprecated pattern with the builder API:

private Bucket createBucket() {
    return Bucket.builder()
        .addLimit(limit -> limit
            .capacity(capacity)
            .refillIntervally(refillTokens, refillPeriod))
        .build();
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/filter/RateLimitingFilter.java` around lines 83 -
91, Replace the deprecated Bandwidth.classic usage inside createBucket(): stop
creating a Bandwidth via Bandwidth.classic(capacity, Refill.intervally(...)) and
instead use the Bucket.builder().addLimit(...) builder API; inside addLimit use
the lambda to set capacity(capacity) and refillIntervally(refillTokens,
refillPeriod) (instead of Refill.intervally), then build the Bucket. Update
references to Bandwidth.classic and keep the existing refillTokens, refillPeriod
and capacity variables.

docs/adr/ADR-001-static-file-serving-architecture.md-4-4 (1)

4-4: ⚠️ Potential issue | 🟡 Minor

ADR status should be updated to reflect current state.

The implementation described in this ADR already exists in the PR (StaticFileHandler, MimeTypeResolver, tests, etc.). Update the status from Proposed to Accepted (or Implemented) so the ADR accurately reflects the decision lifecycle.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/ADR-001-static-file-serving-architecture.md` at line 4, Update the
ADR status line in ADR-001-static-file-serving-architecture.md from "Proposed"
to "Accepted" (or "Implemented") to reflect the actual implementation; ensure
the document header or status field is changed and optionally add a brief note
that StaticFileHandler, MimeTypeResolver and accompanying tests have been
implemented so the ADR accurately matches the existing codebase.

README.md-39-39 (1)

39-39: ⚠️ Potential issue | 🟡 Minor

Multiple fenced code blocks are missing a language identifier (markdownlint MD040).

Lines 39, 141, 156, 162, 174, 269, 322, 364, 370, and 386 open triple-backtick blocks without a language hint. Use text or plaintext for diagrams/paths and bash/yaml/java for the appropriate blocks to silence these MD040 warnings.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 39, Several fenced code blocks in README.md are missing
language identifiers (MD040); open triple-backtick blocks at the noted locations
should be updated to include an appropriate language tag. Edit each offending
block (the triple-backtick fences) and add a language hint such as text or
plaintext for diagrams/paths, bash for shell commands, yaml for YAML snippets,
or java for Java code so the fences (```...) become ```text, ```bash, ```yaml,
etc., to silence the markdownlint MD040 warnings.

docs/adr/ADR-001-static-file-serving-architecture.md-238-238 (1)

238-238: ⚠️ Potential issue | 🟡 Minor

Replace placeholder URL with the actual repository link.

Line 238 contains https://github.com/your-repo/issues/18. Update to the real repository URL before merging.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/ADR-001-static-file-serving-architecture.md` at line 238, Replace
the placeholder repository link "https://github.com/your-repo/issues/18" used in
the markdown link "[Issue `#18`: GET handling for static files]" with the actual
repository issue URL (e.g., "https://github.com/ORG/REPO/issues/18") so the
reference resolves correctly; update the single occurrence of that placeholder
URL in ADR-001-static-file-serving-architecture.md to the real repo path before
merging.

README.md-178-178 (1)

178-178: ⚠️ Potential issue | 🟡 Minor

Heading levels skip from h1 to h3 (markdownlint MD001).

Lines 178 (### Example Mapping) and 251 (### Supported Patterns) follow an h1 section directly — violating the one-level-at-a-time increment rule. Promote the section above to ## or demote the subsection to ## to fix both.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 178, The README uses level-3 headings "### Example
Mapping" and "### Supported Patterns" directly after an h1, causing markdownlint
MD001; update those headings to level-2 (change "### Example Mapping" and "###
Supported Patterns" to "## Example Mapping" and "## Supported Patterns") so the
document increments heading levels one at a time.

src/main/java/org/juv25d/filter/RedirectRule.java-30-35 (1)

30-35: ⚠️ Potential issue | 🟡 Minor

Missing null validation for sourcePath and targetUrl constructor parameters.

If either is null, sourcePath.contains("*") in matches() (or getTargetUrl() usage in RedirectFilter) will NPE at runtime. A simple guard in the constructor prevents this class of bugs:

🛡️ Proposed fix

 public RedirectRule(String sourcePath, String targetUrl, int statusCode) {
     validateStatusCode(statusCode);
+    Objects.requireNonNull(sourcePath, "sourcePath must not be null");
+    Objects.requireNonNull(targetUrl, "targetUrl must not be null");
     this.sourcePath = sourcePath;
     this.targetUrl = targetUrl;
     this.statusCode = statusCode;
 }

Add import java.util.Objects; to the imports.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/filter/RedirectRule.java` around lines 30 - 35, The
RedirectRule constructor lacks null checks for sourcePath and targetUrl which
can cause NPEs in matches() (sourcePath.contains("*")) and when getTargetUrl()
is used; update the RedirectRule(String sourcePath, String targetUrl, int
statusCode) constructor to validate non-null inputs (e.g., using
Objects.requireNonNull or equivalent) for sourcePath and targetUrl before
assigning fields and still call validateStatusCode(statusCode); add the
necessary import (java.util.Objects) if you use Objects.requireNonNull and
ensure error messages clearly state which parameter is null.

src/main/java/org/juv25d/util/ConfigLoader.java-9-12 (1)

9-12: ⚠️ Potential issue | 🟡 Minor

Missing field defaults when YAML sections are absent.

If the server or logging section is entirely absent from application-properties.yml, the null-guard if (serverConfig != null) silently skips the block, leaving port = 0, rootDirectory = null, and logLevel = null. port = 0 causes the server to bind to a random ephemeral port; null values will cause NPEs downstream in ServerLogging and static-file serving.

Initialize the fields to sensible defaults at declaration time so they remain correct even when the YAML section is omitted:

🛡️ Proposed fix

-    private int port;
-    private String logLevel;
-    private String rootDirectory;
+    private int port = 8080;
+    private String logLevel = "INFO";
+    private String rootDirectory = "static";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/util/ConfigLoader.java` around lines 9 - 12, The
ConfigLoader class currently leaves port, logLevel, and rootDirectory
uninitialized when YAML sections are missing, causing port=0 and null NPEs;
initialize the fields at declaration in ConfigLoader (port, logLevel,
rootDirectory) to sensible defaults (e.g., port 8080, logLevel "INFO",
rootDirectory "./public" or another project default) so that when serverConfig
or logging maps are null the rest of the code (including ServerLogging and
static-file serving) has safe fallback values; update any related null-guards in
the init logic to only override these defaults when values are present in the
parsed serverConfig/logging maps.

src/main/java/org/juv25d/http/HttpResponseWriter.java-40-41 (1)

40-41: ⚠️ Potential issue | 🟡 Minor

Content-Length is always emitted, including for status codes that forbid it.

RFC 7230 §3.3.2 prohibits a server from sending Content-Length in 204 No Content and 304 Not Modified responses (and 1xx responses). The current implementation unconditionally writes Content-Length: 0 for these codes, which is a protocol violation that some strict clients or proxies may reject.

🛡️ Suggested guard

+    private static final java.util.Set<Integer> NO_BODY_STATUSES =
+        java.util.Set.of(204, 304);

     private static void writeHeaders(
         OutputStream out,
         Map<String, String> headers,
         byte[] body
     ) throws IOException {
         for (Map.Entry<String, String> header : headers.entrySet()) {
             if (!header.getKey().equalsIgnoreCase("Content-Length")) {
                 String line = header.getKey() + ": " + header.getValue() + "\r\n";
                 out.write(line.getBytes(StandardCharsets.UTF_8));
             }
         }

-        String contentLength = "Content-Length: " + body.length + "\r\n";
-        out.write(contentLength.getBytes(StandardCharsets.UTF_8));
+        // Omit Content-Length for status codes that forbid a message body
+        // (check is done on caller side by passing statusCode here, or gate in write())
         out.write("\r\n".getBytes(StandardCharsets.UTF_8));
     }

The cleanest approach is to pass response.statusCode() into writeHeaders and skip the Content-Length line when the code is 204 or 304.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/http/HttpResponseWriter.java` around lines 40 - 41,
The HttpResponseWriter currently always writes a Content-Length header using the
body byte array; change writeHeaders (or the method that writes headers in
HttpResponseWriter) to accept the response status code (or read
response.statusCode()) and skip emitting the "Content-Length" line when the
status is 1xx, 204, or 304 (i.e., do not write "Content-Length: 0" for those
codes); keep emitting Content-Length for all other statuses and use the existing
body.length logic unchanged for those cases.

docs/adr/ADR-001-static-file-serving-architecture.md-46-46 (1)

46-46: ⚠️ Potential issue | 🟡 Minor

Add language hints to fenced code blocks (markdownlint MD040).

Lines 46, 65, and 224 all open fenced code blocks without a language identifier. markdownlint-cli2 reports MD040 warnings for each. Use text or plaintext for ASCII-art diagrams to silence these.

🔧 Suggested fixes

-```
+```text
 src/main/resources/

(Same fix applies to lines 65 and 224.)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/adr/ADR-001-static-file-serving-architecture.md` at line 46, Several
fenced code blocks in the ADR document open without a language identifier (the
ASCII-art directory/listing blocks); update each of those fenced blocks (the
three places that show plain text/ASCII diagrams) to use a language hint such as
```text or ```plaintext so markdownlint MD040 is satisfied—replace the opening
``` with ```text (e.g., ```text followed by src/main/resources/) for all three
occurrences.

src/test/java/org/juv25d/http/HttpParserTest.java-153-153 (1)

153-153: ⚠️ Potential issue | 🟡 Minor

Use explicit charset in body.getBytes() for platform independence.

body.getBytes() relies on the platform default charset. While "body" is ASCII-safe today, the assertion should be deterministic regardless of JVM locale.

🛡️ Proposed fix

-        assertThat(result.body()).isEqualTo(body.getBytes());
+        assertThat(result.body()).isEqualTo(body.getBytes(StandardCharsets.UTF_8));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/http/HttpParserTest.java` at line 153, The assertion
in HttpParserTest uses body.getBytes() which is platform-dependent; update the
assertion to use an explicit charset (e.g., StandardCharsets.UTF_8) so the
comparison is deterministic across JVM locales—modify the assertion that
references result.body() and body.getBytes() to call
body.getBytes(StandardCharsets.UTF_8) and add the necessary import for
StandardCharsets if missing.

src/test/java/org/juv25d/logging/ServerLoggingTest.java-84-90 (1)

84-90: ⚠️ Potential issue | 🟡 Minor

Test assumes the configured log level is INFO.

This test asserts Level.INFO on the singleton logger, but the actual level is determined by application-properties.yml (or env/system property). If the config specifies a different level, this test breaks. Consider either reading the expected level from the config or testing a freshly configured logger with a known level.

src/test/java/org/juv25d/logging/ServerLoggingTest.java-123-141 (1)

123-141: ⚠️ Potential issue | 🟡 Minor

Add null check for ConfigLoader.getLogLevel() to prevent NPE if configuration is missing.

The logLevel field in ConfigLoader is not initialized with a default value. If the logging.level key is absent from application-properties.yml, line 131 will throw NullPointerException on configLogLevel.toUpperCase(). While the current config file includes the key, the test should be defensive against configuration changes.

Proposed fix

     void logger_shouldUseLogLevelFromApplicationProperties() {
         String configLogLevel = ConfigLoader.getInstance().getLogLevel();
+        assertNotNull(configLogLevel, "Expected logging.level to be set in application-properties.yml");
 
         try {
             System.clearProperty("log.level");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/logging/ServerLoggingTest.java` around lines 123 -
141, The test should defensively handle a missing logging.level by checking
ConfigLoader.getInstance().getLogLevel() for null/blank before calling
toUpperCase(); in the test method
logger_shouldUseLogLevelFromApplicationProperties(), replace the direct use of
configLogLevel with a guarded value (e.g., if configLogLevel is null or empty
set it to "INFO") and then call Level.parse on that guarded string, leaving the
rest of the flow (Logger testLogger, ServerLogging.configure(testLogger),
assertEquals) unchanged so no NPE occurs when the config key is absent.

src/main/java/org/juv25d/Pipeline.java-43-64 (1)

43-64: ⚠️ Potential issue | 🟡 Minor

createChain() should validate that router is set before use.

While setRouter() validates non-null input, createChain() passes router directly to FilterChainImpl without checking if it has been set. This creates an API design gap: if createChain() is called before setRouter(), a NullPointerException will occur in FilterChainImpl.doFilter() (line 27) when router.resolve(req) is invoked after all filters have been processed.

Although the current application always calls setRouter() before processing requests, the defensive check improves API robustness for correct usage:

Proposed fix

     public FilterChainImpl createChain(HttpRequest request) {
+        if (router == null) {
+            throw new IllegalStateException("Router must be set before creating a filter chain");
+        }
         List<Filter> filters = new ArrayList<>();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/juv25d/Pipeline.java` around lines 43 - 64, createChain()
currently passes the field router into new FilterChainImpl without validating
it's been initialized; add a defensive null-check (e.g. use
Objects.requireNonNull(router, ...) or an explicit if (router == null) throw new
IllegalStateException("router not set - call setRouter() before createChain()"))
at the start of createChain() so that calling createChain() before setRouter()
fails fast and clearly (references: createChain, setRouter, router,
FilterChainImpl.doFilter).